-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DPL-1093 - As developers we want to find out how to modify the e2e fixtures so we can re-use the factory wrapper that we are creating so we do not have to duplicate requests #1756
Conversation
Created a printers factory.
…o-find-out-how-to-modify-the-e2e-fixtures-so-we-can-re-use-the-factory-wrapper-that-we-are-creating-so-we-do-not-have-to-duplicate-requests
…o-find-out-how-to-modify-the-e2e-fixtures-so-we-can-re-use-the-factory-wrapper-that-we-are-creating-so-we-do-not-have-to-duplicate-requests
…ility function. Use smrt link factory in pacbioRunCreate store.
Create Pacbio Run factory for use in view. Fix incorrect naming and add some consistency.
…o-find-out-how-to-modify-the-e2e-fixtures-so-we-can-re-use-the-factory-wrapper-that-we-are-creating-so-we-do-not-have-to-duplicate-requests
store.api.v2.traction.printers.get = vi.fn().mockResolvedValue(printerRequestFactory.response) | ||
store.api.v2.traction.printers.get = vi | ||
.fn() | ||
.mockResolvedValue(printersFactory.responses.fetch) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope once we move everything to fetch, it will be just be printersFactory.responses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe. I like the idea of it being descriptive and it gives us the flexibility of using different response types. I think we might need it if we do location tracking as Labwhere does not use JSON API resource.
}, | ||
}, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice abstraction 👍
|
||
const storeData = dataToObjectById({ ...data }) | ||
|
||
return { ...BaseFactory(data), storeData } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope keeping store data will simplify unit tests a lot, as we replicate much of it across tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I decide to add this at this level as each factory will create store data differently. We can see how it progresses ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is best to keep it here because it varies a lot between stores and probably needs many steps for complicated stores. Maybe after we do all these, we might find some patterns and then we can do further abstraction.
…o-find-out-how-to-modify-the-e2e-fixtures-so-we-can-re-use-the-factory-wrapper-that-we-are-creating-so-we-do-not-have-to-duplicate-requests
@seenanair and @dasunpubudumal some stuff was merged into develop this morning so I have had to fix the conflicts. |
Closes #
Changes proposed in this pull request
Instructions for Reviewers
[All PRs] - Confirm PR template filled
[Feature Branches] - Review code
[Production Merges to
main
]- Check story numbers included
- Check for debug code
- Check version